Resolve graph style data once per type instead of once per element - #2119
Closed
kmcginnes wants to merge 2 commits into
Closed
Resolve graph style data once per type instead of once per element#2119kmcginnes wants to merge 2 commits into
kmcginnes wants to merge 2 commits into
Conversation
`LabelPreview` inlined its own `new Color(...).isDark()` with no guard for the empty `labelColor` an imported style file can carry, so the preview could throw where the canvas does not. Both now call `labelTextColorFor`, memoized because parsing a color is the only non-trivial work here. Also reads the fallback from `appDefaultEdgeStyle.labelColor` rather than repeating the hex, and looks up line dash patterns through a `Map` so a `lineStyle` colliding with `Object.prototype` cannot resolve to a function.
Style data varies only by type, so within a render pass N elements of a type now cost one `vertexStyleData` call rather than N. The memoizing closure lives in a plain `create*Resolver` factory because the React Compiler lint rules reject a hook that returns a closure mutating its own captured cache. The vertex resolver takes the styles whose icons are in scope, since the canvas needs only the types it draws while the schema view needs every type. Names the atom lookups (`VertexStyleLookup`, `EdgeStyleLookup`) so that contract is explicit, drops the now-callerless `useAllEdgeStyles`, and tightens the style-context test to assert the exact selector set.
Base automatically changed from
share-label-text-color-helper
to
schema-view-style-perf
August 14, 2026 20:52
kmcginnes
marked this pull request as ready for review
August 14, 2026 20:54
Collaborator
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Style data varies only by type, but was computed once per element. Within a render pass N nodes of a type now cost one
vertexStyleDatacall rather than N.The memoizing closure lives in a plain
create*Resolverfactory rather than inside the hook because the React Compiler lint rules reject a hook that returns a closure mutating its own captured cache. That constraint is non-obvious — a two-line hook forwarding to a factory reads like a pointless wrapper — so it is recorded indocs/agents/react.mdin a later PR in this stack.The vertex resolver takes the styles whose icons are in scope, because the canvas needs only the types it draws while the schema view needs every type. That parameter is what the next PR uses to cut the canvas down from every schema type.
Also here:
VertexStyleLookup/EdgeStyleLookupname the shape of the two style atoms, so the contract is explicit rather than inferred at each call site.useAllEdgeStylesis deleted — it has no callers now thatuseGraphStylesno longer consumes edge styles.toBeLessThanOrEqual(6), which would have passed with three extra selectors.How to read
styleDataResolvers.ts— new; the resolvers and the factories.renderedEntities.ts/useSchemaGraphData.ts— call sites.graphStyles.ts— the lookup types and the dead-code deletion.Stack
Note
A later PR in this stack (
fuse-canvas-vertex-passes) replaces this resolver layer with an eager per-type map, after review found the lazy memoization bought nothing when every caller knows its key set up front. This PR is kept as its own step because the per-type win it introduces is independent of that restructuring and the intermediate state is green. If you would rather not land the resolver layer at all, merge this and the fusing PR together.